Skip to content

Conversation

@takahirom
Copy link
Contributor

@takahirom takahirom commented Dec 11, 2024

As a maintainer of Roborazzi, I've created this Pull Request, but feel free to modify it as you like. Alternatively, you can use it as a reference and implement a different solution, then close this PR—there's no problem with that.

What I Have Done and Why

  • Updated compileSdkVersion and targetSdkVersion to 35.
  • Fixed the theme to match the production app theme's base theme in tests

By default, when using createComposeRule with targetSdkVersion 35, the activity includes an ActionBar. However, the framework does not provide padding for the ActionBar, which results in overlapping screenshots.

This is tracked in this issue.
https://issuetracker.google.com/issues/383368165

In this PR, we can specify the theme by using android:theme for HiltComponentActivity.

As a workaround, we can also specify application android:theme="@android:style/Theme.Material.NoActionBar" for Compose.

@takahirom
Copy link
Contributor Author

takahirom commented Dec 12, 2024

I'm adding a workaround to Roborazzi that uses view.draw(). However, it's a workaround, and I believe using the NoActionBar theme is the preferred solution. Overlapping UI elements in tests can cause issues, such as preventing accessibility checks, not showing the window background in the Activity, and not drawing shadows in Robolectric. Therefore, I think setting the theme like this PR is a better option.

I'm adding a workaround to Roborazzi that uses actionBar.hide() before captureRoboImage() and then calls actionBar.show() when certain conditions are met. However, this is just a workaround and could cause some performance degradation due to layout invalidation. Therefore, I believe the solution in this PR (using the NoActionBar theme) is still preferable.

@takahirom
Copy link
Contributor Author

Since we can reduce the number of screenshot changes, I updated the SDK and changed the theme. If you prefer, I can split this PR. We can change the theme first and then update the SDK to review the screenshot changes separately.

@JoseAlcerreca
Copy link
Contributor

@takahirom thank you so much for this, amazing work! And thanks for filing that bug.

Quick question: why are some screenshots bigger than the references?

@JoseAlcerreca JoseAlcerreca self-assigned this Dec 12, 2024
@takahirom
Copy link
Contributor Author

@JoseAlcerreca
Thank you for checking it out! I think the reason why we have more height in the new screenshots is that we were able to remove the action bar from the activity (which is outside the screenshots).

The old screenshot height is 1403px, and the new screenshot height is 1500px.

image

The activity height might be 1000dp, and the old height might be 936dp.
The difference in dp is 64dp:

(1500 - 1403) / 1.5
= 64.66666666666667

The ActionBar height might be 64dp:
https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/res/res/values-sw600dp/dimens_material.xml;l=23?q=action_bar_default_height_material

I think the width has changed proportionally to the height.

1500 * 561 / 1403
= 599.7861724875268

@dturner dturner self-requested a review December 17, 2024 16:19
@dturner dturner merged commit 4277366 into android:main Dec 17, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants